Fix data uri normalization bug#118
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in data URI normalization by standardizing the handling of data URIs that lack an explicit MIME type. The fix ensures that data URIs starting with "data:," are normalized to "data:text/plain," during initialization rather than being handled as special cases in individual methods.
- Adds input validation to reject non-string arguments
- Normalizes "data:," URIs to "data:text/plain," before regex matching
- Simplifies mimetype and data methods by removing special case handling
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return 'text/plain' | ||
| end | ||
|
|
||
| match[:mimetype] |
There was a problem hiding this comment.
The mimetype method should handle cases where match[:mimetype] is nil or empty. Without the previous fallback logic, this could return nil instead of the expected 'text/plain' default.
| match[:mimetype] | |
| mt = match[:mimetype] | |
| mt.nil? || mt.empty? ? 'text/plain' : mt |
| if normalized_uri.start_with?("data:,") | ||
| normalized_uri = normalized_uri.sub("data:,", "data:text/plain,") | ||
| end |
There was a problem hiding this comment.
The normalization only handles the exact case 'data:,' but should also handle variations with whitespace or parameters like 'data: ,' or 'data:;base64,'. Consider using a more robust pattern match.
| if normalized_uri.start_with?("data:,") | |
| normalized_uri = normalized_uri.sub("data:,", "data:text/plain,") | |
| end | |
| # Normalize any data URI with empty mediatype (with optional whitespace/parameters) to data:text/plain, | |
| normalized_uri = normalized_uri.sub(/\Adata:(\s*(;[a-zA-Z0-9\-]+=[^;,]*)*)?,/, "data:text/plain,") |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| return false unless DataUri.valid?(uri) | ||
|
|
||
| return true if uri.start_with?("data:,{") | ||
|
|
There was a problem hiding this comment.
Bug: Data URI Validation Order Issue
In valid_data_uri?, the DataUri.valid?(uri) check occurs before the special case for data:,{...} URIs. This order means URIs intended for the special handling will likely fail the initial DataUri.valid? check, preventing the dedicated extraction logic from being reached.
Note
Handle
data:,{...}JSON payloads in protocol extractors and add thefacet_rails_commongem.CollectionsParamsExtractorandGenericProtocolExtractor: support raw JSONdata:,{...}URIs by bypassingDataUridecoding for this form.GenericProtocolExtractor#valid_data_uri?: allowdata:,{...}as valid.facet_rails_common(via Git) and its dependencyorder_queryinGemfile.lock.Written by Cursor Bugbot for commit 3922db3. This will update automatically on new commits. Configure here.